Skip to content

fix: remove immer from watch event batching - #49

Merged
joshuapare merged 2 commits into
mainfrom
fix/remove-immer-watch-updates
Mar 23, 2026
Merged

fix: remove immer from watch event batching#49
joshuapare merged 2 commits into
mainfrom
fix/remove-immer-watch-updates

Conversation

@joshuapare

@joshuapare joshuapare commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Wails v3 bindings return class instances (e.g. ListResult) that Immer cannot draft, causing produce can only be called on draftable things errors on every watch event flush
  • Replaced produce() with manual immutable updates (shallow array copy + spread) in useEventBatcher and useWatchState
  • Also eliminates Immer's proxy wrapping and deep-freeze overhead on the hot path
  • Added test case for class instance input to prevent regression

Test plan

  • Verify watch resource updates appear in the UI without Immer errors in the console
  • Confirm no regressions in resource list live updates (ADD/UPDATE/DELETE)
  • Run existing useEventBatcher tests

Summary by CodeRabbit

  • Tests

    • Extended test coverage for resource batching with class instances.
  • Refactor

    • Optimized internal event batching and watch state management logic for improved performance.

…ce errors

Wails v3 bindings return class instances (e.g. ListResult) which Immer
cannot draft, causing "produce can only be called on draftable things"
errors on every watch event flush. Replace produce() with manual
immutable updates (shallow array copy + spread) in useEventBatcher and
useWatchState. This also removes Immer's proxy wrapping and deep-freeze
overhead on the hot path.
@coderabbitai

coderabbitai Bot commented Mar 23, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@joshuapare has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 11 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 24160190-2591-43d3-8234-d6fe5a3d768c

📥 Commits

Reviewing files that changed from the base of the PR and between ef52283 and 49cee2c.

📒 Files selected for processing (1)
  • packages/omniviewdev-runtime/src/hooks/resource/useWatchState.ts
📝 Walkthrough

Walkthrough

Removed Immer dependency from event batching and watch state management. Replaced Immer's produce function with explicit immutable-style updates using object and array spreading. Added test coverage for applyBatch with class-instance objects.

Changes

Cohort / File(s) Summary
Event Batcher Refactoring
useEventBatcher.ts, useEventBatcher.test.ts
Replaced Immer-based draft mutations with explicit immutable updates (shallow array copy, conditional item appending/replacing/removal for ADD/UPDATE/DELETE events). Extended test coverage to validate applyBatch handles class-instance oldData objects.
Watch State Refactoring
useWatchState.ts
Removed Immer draft mutations from applyEvent. Replaced with explicit object spreading for resources and resourceCounts updates. Aggregate recomputation and return value now use spread-based object reconstruction instead of draft mutation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 With Immer gone, we spread with care,
Objects cloned through every layer fair,
ADD, UPDATE, DELETE all align,
Immutable patterns, tried and fine! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: removing Immer from watch event batching logic across useEventBatcher and useWatchState.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/remove-immer-watch-updates

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/omniviewdev-runtime/src/hooks/resource/useWatchState.ts`:
- Around line 44-56: The updater currently recomputes resources, resourceCounts,
syncedCount, and errorCount but leaves totalResources stale by spreading ...old;
update the returned object to set totalResources to the current number of
resource keys (e.g., Object.keys(resources).length) so
WatchConnectionSummary.totalResources reflects the new resources map; locate the
updater in useWatchState.ts where resources is built and include totalResources
alongside resources, resourceCounts, syncedCount, and errorCount in the returned
object.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3a9420e6-a150-41e6-9158-5cf8c2fe4c2a

📥 Commits

Reviewing files that changed from the base of the PR and between dd6cd05 and ef52283.

📒 Files selected for processing (3)
  • packages/omniviewdev-runtime/src/hooks/resource/useEventBatcher.test.ts
  • packages/omniviewdev-runtime/src/hooks/resource/useEventBatcher.ts
  • packages/omniviewdev-runtime/src/hooks/resource/useWatchState.ts

Comment thread packages/omniviewdev-runtime/src/hooks/resource/useWatchState.ts Outdated
totalResources was inherited from the old state via spread, so it became
stale when a new resource key arrived via a watch event. This affected
polling decisions, isFullySynced, and syncProgress.
@joshuapare
joshuapare merged commit 696ec38 into main Mar 23, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant